Skip to content

bugfix: enhance AddKnowledgeDialog with scrollable content and fixed header#760

Closed
seanrobertwright wants to merge 1 commit intocoleam00:mainfrom
seanrobertwright:dialog-fix
Closed

bugfix: enhance AddKnowledgeDialog with scrollable content and fixed header#760
seanrobertwright wants to merge 1 commit intocoleam00:mainfrom
seanrobertwright:dialog-fix

Conversation

@seanrobertwright
Copy link
Copy Markdown

@seanrobertwright seanrobertwright commented Oct 4, 2025

I hope the noob (me) did this correctly...
Addresses bug #759 https://github.com/coleam00/Archon/issues/759

  • Introduced ScrollableDialogContent component for better user experience in AddKnowledgeDialog.
  • Updated dialog structure to include a fixed header and scrollable body, improving accessibility and usability.
  • Adjusted styles to maintain visual consistency and added maxHeight prop for dynamic height management.
  • Updated .gitignore to include DIALOG-FIX.md for better tracking of dialog-related changes.

This change enhances the Add Knowledge dialog's functionality, making it more user-friendly while adhering to design principles.

Pull Request

Summary

Dialog Overflow Fix Proposal

Problem Analysis

The AddKnowledgeDialog currently exceeds window boundaries and gets cut off on smaller screens or when the dialog content is too tall. The issue stems from:

  1. Fixed positioning: Dialog uses fixed left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2 which centers it but doesn't respect viewport boundaries
  2. No height constraints: Dialog content can grow indefinitely without scroll handling
  3. Missing responsive behavior: No fallback for smaller screens or overflow scenarios

Root Cause

Current DialogContent implementation (lines 42-62 in dialog.tsx):

className={cn(
  "fixed left-1/2 top-1/2 z-50 -translate-x-1/2 -translate-y-1/2",
  "p-6 rounded-md backdrop-blur-md",
  "w-full max-w-2xl", // Only width constraint, no height constraint
  // ... styling classes
)}

AddKnowledgeDialog usage (line 130):

<DialogContent className="sm:max-w-[600px]"> // Still no height constraint

Proposed Solution

1. Enhanced DialogContent Component

Modify DialogContent to include responsive height constraints and scroll behavior:

// In dialog.tsx - Enhanced DialogContent
export const DialogContent = React.forwardRef<
  React.ElementRef<typeof DialogPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> & {
    showCloseButton?: boolean;
    maxHeight?: string; // New prop for custom height constraints
  }
>(({ className, children, showCloseButton = true, maxHeight = "90vh", ...props }, ref) => (
  <DialogPortal>
    <DialogOverlay />
    <DialogPrimitive.Content
      ref={ref}
      className={cn(
        "fixed left-1/2 top-1/2 z-50 -translate-x-1/2 -translate-y-1/2",
        "p-6 rounded-md backdrop-blur-md",
        "w-full max-w-2xl",
        // NEW: Height constraints and scroll behavior
        `max-h-[${maxHeight}]`, // Configurable max height
        "overflow-hidden", // Prevent content from breaking out
        // ... existing styling classes
        className,
      )}
      style={{ maxHeight }} // Ensure maxHeight prop works
      {...props}
    >
      <div className="relative z-10 h-full flex flex-col">
        {children}
      </div>
      {showCloseButton && (
        <DialogPrimitive.Close
          className={cn(
            "absolute right-4 top-4 z-20",
            "text-gray-500 dark:text-gray-400",
            "hover:text-gray-700 dark:hover:text-white",
            "transition-colors",
          )}
        >
          <X className="h-5 w-5" />
          <span className="sr-only">Close</span>
        </DialogPrimitive.Close>
      )}
    </DialogPrimitive.Content>
  </DialogPortal>
));

2. New ScrollableDialogContent Component

Create a specialized component for dialogs that need scrollable content:

// In dialog.tsx - New ScrollableDialogContent
export const ScrollableDialogContent = React.forwardRef<
  React.ElementRef<typeof DialogPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> & {
    showCloseButton?: boolean;
    maxHeight?: string;
  }
>(({ className, children, showCloseButton = true, maxHeight = "90vh", ...props }, ref) => (
  <DialogPortal>
    <DialogOverlay />
    <DialogPrimitive.Content
      ref={ref}
      className={cn(
        "fixed left-1/2 top-1/2 z-50 -translate-x-1/2 -translate-y-1/2",
        "rounded-md backdrop-blur-md",
        "w-full max-w-2xl",
        `max-h-[${maxHeight}]`,
        "overflow-hidden flex flex-col",
        // ... existing styling classes
        className,
      )}
      style={{ maxHeight }}
      {...props}
    >
      {/* Fixed Header */}
      <div className="flex-shrink-0 p-6 pb-4">
        {children}
      </div>
      
      {/* Scrollable Body */}
      <div className="flex-1 overflow-y-auto px-6 pb-6">
        {/* Content goes here - this will be scrollable */}
      </div>
      
      {showCloseButton && (
        <DialogPrimitive.Close
          className={cn(
            "absolute right-4 top-4 z-20",
            "text-gray-500 dark:text-gray-400",
            "hover:text-gray-700 dark:hover:text-white",
            "transition-colors",
          )}
        >
          <X className="h-5 w-5" />
          <span className="sr-only">Close</span>
        </DialogPrimitive.Close>
      )}
    </DialogPrimitive.Content>
  </DialogPortal>
));

3. Enhanced AddKnowledgeDialog Implementation

Modify AddKnowledgeDialog to use the new scrollable component:

// In AddKnowledgeDialog.tsx
return (
  <Dialog open={open} onOpenChange={onOpenChange}>
    <ScrollableDialogContent className="sm:max-w-[600px]" maxHeight="85vh">
      {/* Fixed Header */}
      <DialogHeader>
        <DialogTitle>Add Knowledge</DialogTitle>
        <DialogDescription>Crawl websites or upload documents to expand your knowledge base.</DialogDescription>
      </DialogHeader>

      {/* Scrollable Content */}
      <div className="overflow-y-auto">
        <Tabs value={activeTab} onValueChange={(v) => setActiveTab(v as "crawl" | "upload")}>
          {/* Enhanced Tab Buttons */}
          <div className="grid grid-cols-2 gap-3 p-2 rounded-xl backdrop-blur-md bg-gradient-to-b from-gray-100/30 via-gray-50/20 to-white/40 dark:from-gray-900/30 dark:via-gray-800/20 dark:to-black/40 border border-gray-200/40 dark:border-gray-700/40">
            {/* ... existing tab buttons */}
          </div>

          {/* Crawl Tab */}
          <TabsContent value="crawl" className="space-y-6 mt-6">
            {/* ... existing crawl content */}
          </TabsContent>

          {/* Upload Tab */}
          <TabsContent value="upload" className="space-y-6 mt-6">
            {/* ... existing upload content */}
          </TabsContent>
        </Tabs>
      </div>
    </ScrollableDialogContent>
  </Dialog>
);

4. Alternative: CSS-Only Solution

If you prefer a simpler approach, modify only the existing DialogContent:

// In dialog.tsx - Simple CSS-only fix
export const DialogContent = React.forwardRef<
  React.ElementRef<typeof DialogPrimitive.Content>,
  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> & {
    showCloseButton?: boolean;
  }
>(({ className, children, showCloseButton = true, ...props }, ref) => (
  <DialogPortal>
    <DialogOverlay />
    <DialogPrimitive.Content
      ref={ref}
      className={cn(
        "fixed left-1/2 top-1/2 z-50 -translate-x-1/2 -translate-y-1/2",
        "p-6 rounded-md backdrop-blur-md",
        "w-full max-w-2xl",
        // NEW: Height constraints and scroll
        "max-h-[90vh]", // Constrain to 90% of viewport height
        "overflow-y-auto", // Enable vertical scrolling
        // ... existing styling classes
        className,
      )}
      {...props}
    >
      <div className="relative z-10">{children}</div>
      {/* ... existing close button */}
    </DialogPrimitive.Content>
  </DialogPortal>
));

Implementation Steps

Option A: Enhanced Components (Recommended AND IMPLEMENTED)

  1. Update dialog.tsx:

    • Add ScrollableDialogContent component
    • Enhance DialogContent with optional height constraints
    • Maintain backward compatibility
  2. Update AddKnowledgeDialog.tsx:

    • Replace DialogContent with ScrollableDialogContent
    • Restructure content into fixed header and scrollable body
    • Test on various screen sizes
  3. Test & Validate:

    • Test on mobile devices (320px width)
    • Test with long content (many tags, long URLs)
    • Verify scroll behavior is smooth
    • Ensure close button remains accessible

Option B: CSS-Only Fix (Quick Solution)

  1. Update dialog.tsx:

    • Add max-h-[90vh] and overflow-y-auto to DialogContent
    • No structural changes needed
  2. Test:

    • Verify scroll behavior
    • Check mobile responsiveness

Benefits

  • Responsive: Works on all screen sizes
  • Accessible: Maintains keyboard navigation and focus management
  • Consistent: Preserves existing design aesthetics
  • Flexible: Configurable height constraints
  • Performance: Minimal impact on existing functionality

Considerations

  • Close button: Ensure it remains visible and accessible
  • Scroll indicators: Consider adding subtle scroll indicators
  • Mobile optimization: May need additional mobile-specific adjustments
  • Backward compatibility: Existing dialogs should continue working

Testing Checklist

Manual Tests:

  • Dialog fits within viewport on 320px width screens
  • Scroll behavior is smooth and intuitive
  • Close button remains accessible when scrolled
  • Tab navigation works correctly when scrolled
  • Form validation and submission work correctly
  • Keyboard navigation (Tab, Enter, Escape) works properly
  • Screen reader accessibility is maintained
  • Visual design remains consistent

Recommended Approach

Use Option A (Enhanced Components) for the following reasons:

  1. Better UX: Fixed header keeps title/description visible
  2. Maintainable: Clear separation of concerns
  3. Reusable: Can be used for other large dialogs
  4. Future-proof: Easier to extend with additional features

The enhanced approach provides better user experience while maintaining the existing design system and ensuring accessibility standards are met.

Changes Made

  1. Update dialog.tsx:

    • Add ScrollableDialogContent component
    • Enhance DialogContent with optional height constraints
    • Maintain backward compatibility
  2. Update AddKnowledgeDialog.tsx:

    • Replace DialogContent with ScrollableDialogContent
    • Restructure content into fixed header and scrollable body
    • Test on various screen sizes
  3. Test & Validate:

    • Test on mobile devices (320px width)
    • Test with long content (many tags, long URLs)
    • Verify scroll behavior is smooth
    • Ensure close button remains accessible

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

image

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

Additional Notes

Summary by CodeRabbit

  • New Features

    • Introduced a scrollable dialog option with configurable maximum height and optional close button for improved usability on smaller screens.
  • Refactor

    • Updated the Add Knowledge dialog to use a fixed header and a scrollable content area, improving readability and navigation without altering existing crawl or upload functionality.
    • Adjusted tab layout and helper text styling to align with the new scrollable structure.
  • Chores

    • Updated ignored files to exclude DIALOG-FIX.md.

…ader

- Introduced ScrollableDialogContent component for better user experience in AddKnowledgeDialog.
- Updated dialog structure to include a fixed header and scrollable body, improving accessibility and usability.
- Adjusted styles to maintain visual consistency and added maxHeight prop for dynamic height management.
- Updated .gitignore to include DIALOG-FIX.md for better tracking of dialog-related changes.

This change enhances the dialog's functionality, making it more user-friendly while adhering to design principles.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 4, 2025

Walkthrough

Introduces a new ScrollableDialogContent UI primitive and updates DialogContent to accept an optional maxHeight prop. Refactors AddKnowledgeDialog to use ScrollableDialogContent with a fixed header and scrollable body. .gitignore adds DIALOG-FIX.md to ignored files.

Changes

Cohort / File(s) Summary
Repo config
`.gitignore`
Ignore `DIALOG-FIX.md`.
UI primitives: Dialog
`archon-ui-main/src/features/ui/primitives/dialog.tsx`
Added exported `ScrollableDialogContent` component with scrollable body and optional close button; extended `DialogContent` props to include `maxHeight?: string`.
Knowledge: AddKnowledgeDialog layout
`archon-ui-main/src/features/knowledge/components/AddKnowledgeDialog.tsx`
Switched from `DialogContent` to `ScrollableDialogContent`. Introduced fixed header block and scrollable main area containing Tabs (Crawl/Upload) and inputs. Updated imports and container classes; no logic changes to crawl/upload flows.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant AddKnowledgeDialog
  participant UI_Primitives as ScrollableDialogContent
  participant Tabs as Tabs/Crawl/Upload

  User->>AddKnowledgeDialog: Open dialog
  AddKnowledgeDialog->>UI_Primitives: Render with maxHeight (85–90vh)
  Note over UI_Primitives: Fixed header rendered<br/>Scrollable body area
  UI_Primitives->>AddKnowledgeDialog: Slot header + body
  AddKnowledgeDialog->>Tabs: Render tabs inside scrollable body
  User->>Tabs: Select "Crawl" or "Upload"
  Tabs-->>AddKnowledgeDialog: Update active tab content
  User->>AddKnowledgeDialog: Submit action (unchanged handlers)
  AddKnowledgeDialog-->>User: Success/Error (existing flow)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • coleam00
  • Wirasm

Poem

A hop, a scroll, a tidy view,
I pinned the header—sky stays blue.
Tabs that glide, content that flows,
Burrows of bytes where knowledge grows.
With gentle hops I test and gleam—
Now dialogs match my rabbit dream. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title concisely and accurately summarizes the primary change by highlighting the enhancement of AddKnowledgeDialog with scrollable content and a fixed header. It is directly related to the main change, clear, and free of unnecessary details.
Description Check ✅ Passed The PR description closely follows the repository’s template, providing all required sections including Summary, Changes Made, Type of Change, Affected Services, Testing with evidence, Checklist, Breaking Changes, and Additional Notes. Each section is populated with relevant and specific information, and the required checkboxes and evidence are present. The additional analysis and solution details complement but do not duplicate the template structure, making the description both thorough and compliant.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@seanrobertwright seanrobertwright changed the title feat: enhance AddKnowledgeDialog with scrollable content and fixed header bugfix: enhance AddKnowledgeDialog with scrollable content and fixed header Oct 4, 2025
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63a92cf and b627e7d.

📒 Files selected for processing (3)
  • .gitignore (1 hunks)
  • archon-ui-main/src/features/knowledge/components/AddKnowledgeDialog.tsx (2 hunks)
  • archon-ui-main/src/features/ui/primitives/dialog.tsx (4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
archon-ui-main/src/features/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

archon-ui-main/src/features/**/*.{ts,tsx}: Use TanStack Query for all data fetching (no prop drilling); use smart HTTP polling (no WebSockets)
Biome formatting in features: 120-character line length, double quotes, and trailing commas
Apply Tron-inspired glassmorphism styling with Tailwind for feature UI

Use Biome in features: 120 character line length, double quotes, and trailing commas

Files:

  • archon-ui-main/src/features/ui/primitives/dialog.tsx
  • archon-ui-main/src/features/knowledge/components/AddKnowledgeDialog.tsx
archon-ui-main/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Disallow implicit any in TypeScript

archon-ui-main/src/**/*.{ts,tsx}: Frontend TypeScript must use strict mode with no implicit any
Use TanStack Query for all data fetching; avoid prop drilling
Use database values directly in the frontend; avoid mapping layers between BE and FE types

Files:

  • archon-ui-main/src/features/ui/primitives/dialog.tsx
  • archon-ui-main/src/features/knowledge/components/AddKnowledgeDialog.tsx
archon-ui-main/src/features/ui/primitives/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Radix UI primitives from the primitives library when building feature components

Use Radix UI primitives from src/features/ui/primitives when creating UI components

Files:

  • archon-ui-main/src/features/ui/primitives/dialog.tsx
archon-ui-main/src/**/*.{ts,tsx,py}

📄 CodeRabbit inference engine (CLAUDE.md)

In code comments, avoid meta terms like SIMPLIFIED, ENHANCED, LEGACY, CHANGED, REMOVED; comment only on functionality and reasoning (do not mention beta/global rules)

Files:

  • archon-ui-main/src/features/ui/primitives/dialog.tsx
  • archon-ui-main/src/features/knowledge/components/AddKnowledgeDialog.tsx
archon-ui-main/src/features/*/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place new UI components under the feature’s components directory

Place new UI components under src/features/[feature]/components

Files:

  • archon-ui-main/src/features/knowledge/components/AddKnowledgeDialog.tsx
🧬 Code graph analysis (2)
archon-ui-main/src/features/ui/primitives/dialog.tsx (1)
archon-ui-main/src/features/ui/primitives/styles.ts (1)
  • cn (139-141)
archon-ui-main/src/features/knowledge/components/AddKnowledgeDialog.tsx (4)
archon-ui-main/src/features/ui/primitives/dialog.tsx (4)
  • ScrollableDialogContent (90-147)
  • DialogHeader (151-155)
  • DialogTitle (171-185)
  • DialogDescription (189-198)
archon-ui-main/src/features/ui/primitives/tabs.tsx (2)
  • Tabs (6-6)
  • TabsContent (90-102)
archon-ui-main/src/features/ui/primitives/styles.ts (1)
  • cn (139-141)
archon-ui-main/src/features/ui/primitives/input.tsx (2)
  • Label (67-81)
  • Input (8-29)

Comment on lines +127 to +145
{/* Scrollable Content Area */}
<div className="flex-1 overflow-y-auto">
{children}
</div>

{showCloseButton && (
<DialogPrimitive.Close
className={cn(
"absolute right-4 top-4 z-20",
"text-gray-500 dark:text-gray-400",
"hover:text-gray-700 dark:hover:text-white",
"transition-colors",
)}
>
<X className="h-5 w-5" />
<span className="sr-only">Close</span>
</DialogPrimitive.Close>
)}
</DialogPrimitive.Content>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Header still scrolls—wrapper needs restructuring

Wrapping every child inside div.flex-1.overflow-y-auto forces the header consumers pass (e.g., AddKnowledgeDialog) into the scrolling region, so the “fixed” header still scrolls away. Please let the parent render a flex column and allow callers to supply their own non-scrolling header + scrolling body split.

Apply this diff to keep the layout flexible and actually support a fixed header:

-      {/* Scrollable Content Area */}
-      <div className="flex-1 overflow-y-auto">
-        {children}
-      </div>
+      <div className="relative z-10 flex h-full flex-col">
+        {children}
+      </div>
🤖 Prompt for AI Agents
In archon-ui-main/src/features/ui/primitives/dialog.tsx around lines 127 to 145,
the current implementation wraps every child in a div.flex-1.overflow-y-auto
which forces headers passed by consumers into the scrolling region; remove that
catch-all wrapper and instead render a flex column container where the component
does not assume every child is scrollable—provide a slot/prop expectation or a
specific body wrapper: render children directly (so callers can place a
non-scrolling header and a scrollable body themselves) or accept separate header
and body props and only wrap the body in div.flex-1.overflow-y-auto; keep the
Close button placement unchanged so it remains absolutely positioned.

@coleam00
Copy link
Copy Markdown
Owner

coleam00 commented Oct 5, 2025

Thank you for this! @leex279 could you please take a look at see if this is good on your end?

@leex279 leex279 self-requested a review November 7, 2025 14:04
@Wirasm
Copy link
Copy Markdown
Collaborator

Wirasm commented Apr 6, 2026

🔄 This repository is being replaced by a new version of Archon.

The original Python/MCP codebase is being archived to the archive/v1-python-mcp branch. The new Archon (TypeScript workflow engine for AI coding agents) is replacing it.

This PR is being closed as part of the migration. Thank you for your contribution!

@Wirasm Wirasm closed this Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants